Skip to content

test: optimize and de-brittle test suite - #100

Merged
Snuffy2 merged 16 commits into
mainfrom
test/optimize-suite
Aug 9, 2026
Merged

test: optimize and de-brittle test suite#100
Snuffy2 merged 16 commits into
mainfrom
test/optimize-suite

Conversation

@Snuffy2

@Snuffy2 Snuffy2 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

Streamlines the test suite by combining behaviorally equivalent cases and removing assertions coupled to internal implementation details.

What Changed

  • Parameterized repeated CLI error mappings, validation, endpoint, duration, SMART, and traffic cases.
  • Preserved public behavior coverage for error handling, API endpoints, and redaction while consolidating overlapping tests.
  • Removed brittle checks on caller frames, JSON decoder inputs, private-helper call counts, and fixed availability-probe ordering.

Why

The suite now focuses on observable behavior, reducing maintenance churn from harmless refactors while retaining regression coverage.

Summary by CodeRabbit

  • Bug Fixes

    • Improved DHCP lease expiration handling across time zones.
    • Improved consistency when interpreting traffic statistics and date-based reports in UTC.
    • Preserved reliable event-stream parsing, firewall operations, device selection, and reboot/Wake-on-LAN results.
  • Refactor

    • Simplified internal success handling and validation while preserving existing behavior.
    • Improved type clarity and code consistency across the client, scripts, and documentation tooling.
  • Tests

    • Consolidated and expanded coverage for error handling, time zones, parsing, scripts, and endpoint behavior.

Greptile Summary

The PR consolidates repetitive tests, removes assertions coupled to implementation details, and applies behavior-preserving cleanup across the client and scripts.

  • Parameterizes repeated validation, error-mapping, endpoint, duration, SMART, and traffic cases.
  • Normalizes timezone handling for DHCP leases and vnStat date parsing.
  • Simplifies equivalent conditionals, type annotations, imports, and stream parsing.
  • Adds compliant docstrings to the previously flagged nested test helpers.

Confidence Score: 4/5

The PR is not yet safe to merge because the outstanding delayed-import issue causes the configured Ruff check to fail.

The two live scripts still place imports after executable module statements without E402 suppressions, while the repository’s Ruff configuration provides no applicable exclusion or per-file ignore.

Files Needing Attention: scripts/aiopnsense_dump.py and scripts/opnsense_api_call.py

Important Files Changed

Filename Overview
scripts/aiopnsense_dump.py Removes E402 suppressions while leaving imports below an executable module block, so the previously reported Ruff failure remains.
scripts/opnsense_api_call.py Leaves the delayed aiohttp import unsuppressed after executable module code, sharing the outstanding Ruff failure.
tests/test_scripts_aiopnsense_dump.py Consolidates CLI error tests and now documents the nested error helper in the required Google style.
tests/test_scripts_opnsense_api_call.py Consolidates expected CLI failures and now provides a compliant docstring for the nested helper.
aiopnsense/dhcp.py Attaches the configured OPNsense timezone while parsing lease expiration values without changing the resulting aware datetime.
aiopnsense/client_transport.py Replaces the explicit single-space SSE prefix removal with equivalent removeprefix behavior.

Reviews (3): Last reviewed commit: "test: document parameterized CLI error h..." | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request applies formatting and typing cleanup, updates timezone-aware parsing and conversions, simplifies status returns, and consolidates tests through parametrization and direct mocks.

Changes

Library behavior and tooling

Layer / File(s) Summary
Runtime parsing and status handling
aiopnsense/*, docs/source/*, scripts/*
The library updates SSE parsing, DHCP timestamp construction, status returns, device selection, vnstat conversions, UTC handling, annotations, imports, and lint placement.
Client and transport test consolidation
tests/conftest.py, tests/test_client_*.py, tests/test_client_transport.py, tests/test_dhcp.py, tests/test_exceptions.py, tests/test_firewall.py
Client and transport tests use parametrization, direct mocks, simplified fixtures, and updated annotations.
Helper and script test updates
tests/test_helpers.py, tests/test_nut.py, tests/test_scripts_*
Helper and script tests consolidate error cases, update async typing, and replace generated fixture strings with multiline literals.
Feature regression test coverage
tests/test_smart.py, tests/test_speedtest.py, tests/test_system.py, tests/test_traffic.py, tests/test_vnstat.py
SMART, speedtest, system, traffic, and vnstat tests align with the updated behavior and remove redundant assertions.
Estimated code review effort: 2 (Simple) ~15 minutes

Suggested labels: code-quality

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 91.30% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: streamlining and reducing brittleness in the test suite.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@read-the-docs-community

read-the-docs-community Bot commented Aug 8, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  aiopnsense
  client.py
  client_transport.py
  dhcp.py
  firewall.py
  nut.py
  system.py
  traffic.py
  vnstat.py 488, 508
Project Total  

This report was generated by python-coverage-comment-action

Comment thread scripts/aiopnsense_dump.py
Comment thread tests/test_scripts_aiopnsense_dump.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/source/conf.py`:
- Line 24: Keep the Sphinx configuration key named copyright in the
configuration assignment, and add a targeted Ruff A001 suppression for that line
rather than renaming the key.

In `@tests/test_client_endpoint.py`:
- Around line 974-999: The boolean parameters expected_use_snake_case in
tests/test_client_endpoint.py:974-999 and expected in tests/test_dhcp.py:295-314
trigger Ruff FBT001; update both test parameter definitions to follow the
repository’s existing style by making them keyword-only, or add a justified
local # noqa: FBT001 suppression where that style requires positional
parameters.

In `@tests/test_scripts_aiopnsense_dump.py`:
- Around line 972-973: Add Google Style docstrings to both nested raise_error
helpers: tests/test_scripts_aiopnsense_dump.py lines 972-973 and
tests/test_scripts_opnsense_api_call.py lines 836-837. Document that each helper
raises the parameterized CLI failure created by error_factory(module).
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3bdee3eb-3ca3-435e-98ec-a1751b1dd692

📥 Commits

Reviewing files that changed from the base of the PR and between d42fcce and 0e56945.

📒 Files selected for processing (32)
  • aiopnsense/__init__.py
  • aiopnsense/client.py
  • aiopnsense/client_transport.py
  • aiopnsense/dhcp.py
  • aiopnsense/firewall.py
  • aiopnsense/nut.py
  • aiopnsense/system.py
  • aiopnsense/traffic.py
  • aiopnsense/vnstat.py
  • docs/source/_ext/opnsense_client_api.py
  • docs/source/conf.py
  • scripts/_opnsense_live_common.py
  • scripts/aiopnsense_dump.py
  • scripts/opnsense_api_call.py
  • tests/conftest.py
  • tests/test_client_base.py
  • tests/test_client_endpoint.py
  • tests/test_client_queue.py
  • tests/test_client_transport.py
  • tests/test_dhcp.py
  • tests/test_exceptions.py
  • tests/test_firewall.py
  • tests/test_helpers.py
  • tests/test_nut.py
  • tests/test_scripts_aiopnsense_dump.py
  • tests/test_scripts_live_common.py
  • tests/test_scripts_opnsense_api_call.py
  • tests/test_smart.py
  • tests/test_speedtest.py
  • tests/test_system.py
  • tests/test_traffic.py
  • tests/test_vnstat.py

Comment thread docs/source/conf.py
Comment thread tests/test_client_endpoint.py
Comment thread tests/test_scripts_aiopnsense_dump.py
@Snuffy2
Snuffy2 merged commit 5d435d3 into main Aug 9, 2026
13 checks passed
@Snuffy2
Snuffy2 deleted the test/optimize-suite branch August 9, 2026 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant